Skip to content

Remove LineString upcast when accessing Polygon rings#290

Merged
dr-jts merged 2 commits into
locationtech:masterfrom
dbaston:remove-ring-access-upcast
Nov 21, 2018
Merged

Remove LineString upcast when accessing Polygon rings#290
dr-jts merged 2 commits into
locationtech:masterfrom
dbaston:remove-ring-access-upcast

Conversation

@dbaston

@dbaston dbaston commented Jun 12, 2018

Copy link
Copy Markdown
Contributor

This makes it simpler for a user to construct new Polygons from one or more rings of an existing Polygon.

Do this to make it simpler for a user to construct new Polygons from one
or more rings of an existing Polygon.

Signed-off-by: Daniel Baston <dbaston@gmail.com>
@dr-jts

dr-jts commented Jun 12, 2018

Copy link
Copy Markdown
Contributor

The use of LineString as the return type was done to follow the OGC Simple Features Specification (see OGC SFS 1.1.0 Section 6.1.11.2). Not sure why they used LineString rather than LinearRing.

Any comments on whether it's better to be spec-compliant, or easier to use?

@dbaston

dbaston commented Jun 12, 2018

Copy link
Copy Markdown
Contributor Author

Hmm, I figured as much. I'll weasel around the tough question and make the argument that the PR is spec-compliant, because a LinearRing is indeed a LineString.

@dr-jts

dr-jts commented Jun 12, 2018

Copy link
Copy Markdown
Contributor

That seems reasonable to me. Still odd that they didn't do this in the spec - but I can't see a reason for this, so perhaps it just slipped through the cracks (it's the kind of thing that might not be noticed until people actually started using the spec).

Signed-off-by: Daniel Baston <dbaston@gmail.com>
@dbaston

dbaston commented Nov 21, 2018

Copy link
Copy Markdown
Contributor Author

@dr-jts can this be merged, or should I close it out?

@dr-jts dr-jts merged commit 8ba2900 into locationtech:master Nov 21, 2018
@jodygarnett

Copy link
Copy Markdown
Contributor

This has broken geotools build:

@jodygarnett

jodygarnett commented Feb 15, 2019

Copy link
Copy Markdown
Contributor

Looking at failure in downstream application:

Exception in thread "SunTileScheduler0Standard1" java.lang.NoSuchMethodError: org.locationtech.jts.geom.Polygon.getExteriorRing()Lorg/locationtech/jts/geom/LineString;
	at it.geosolutions.jaiext.utilities.shape.PolygonIterator.<init>(PolygonIterator.java:61)
	at it.geosolutions.jaiext.utilities.shape.GeomCollectionIterator.getIterator(GeomCollectionIterator.java:92)
	at it.geosolutions.jaiext.utilities.shape.GeomCollectionIterator.init(GeomCollectionIterator.java:65)
	at it.geosolutions.jaiext.utilities.shape.GeomCollectionIterator.<init>(GeomCollectionIterator.java:75)
	at it.geosolutions.jaiext.utilities.shape.LiteShape.getPathIterator(LiteShape.java:245)

It appears that the exact method signature has been baked into the resulting byte code as if this was a final method.

Some research is needed to see if narrowing the return type is a breaking change in java.

@jodygarnett

Copy link
Copy Markdown
Contributor

Okay because this was an API change we should of updated the version number to 1.17.0-SNAPSHOT.

I am going to see if I can pull over everything except this to the 1.16.x branch

@dr-jts

dr-jts commented Feb 16, 2019

Copy link
Copy Markdown
Contributor

@jodygarnett There have been a lot of changes since 1.16. To cut a 1.16.1 should probably just pull over the commits fixing CoordSequences and leave it at that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants